install: give a tarball row that is read again the lifecycle scripts of its build by robobun · Pull Request #46238 · oven-sh/bun

robobun · 2026-10-20T20:06:11Z

Problem

bun update, bun install --force and bun add <the same spec> read a file: or URL tarball again. Its lockfile row moves to the new build with the old lifecycle scripts and hasInstallScript.
With bun.lockb a trusted package runs the old postinstall. With bun.lock the install prints no Blocked 1 postinstall.
append_extracted_package (src/install/PackageManager/processDependencyList.rs:198) copies dependencies, bin and the integrity, not scripts or has_install_script.

Fix

A tarball row that is read again takes has_install_script, and scripts when it records scripts (bun.lockb). The same bytes repair an older row.
A git or GitHub row that bun update reads again only gains a missing hasInstallScript.
Verified: test/cli/install/bun-install-tarball-integrity.test.ts, 29 cases. Main fails 25. Each clause removed alone fails 1 to 16.
Self-reviewed: 37 concerns raised, 35 addressed. Rejected: a git case that runs a hook (no scripts are stored there), a whole-row store (the row keeps resolution).

Background

A tarball row moves when its recorded integrity is not the sha512 of what was read.
bun.lock records no scripts: the installer reads the installed package.json, which a patch can edit. hasInstallScript gates the count of blocked scripts.
Considered an unconditional scripts store. With a patch that edits scripts, 20 of 20 bun.lock cells run the unpatched hook.

Downsides

The linkers that give a row its first integrity (PackageInstaller.rs:1574, isolated_install/Installer.rs:293) still record nothing else.
bun update adds hasInstallScript: true to tarball, git and GitHub rows of an older bun.lock: one changed line per row.
Size: +635 bytes in append_extracted_package, +768 bytes of text (release).

Notes

Origin. An audit found this, no user reported it. The change replaces commit 1eeca05 of draft #45671, which that PR's self-review asked to have as a PR of its own. #45671 can drop that commit. That commit stored both fields on every row that moves. This one stores them on tarball rows only.

History.

The bun.lockb half is older than install: consolidate the open package manager pull requests #44968. With the hoisted linker, 1.4.3-canary.1 already lays the new build and runs the old postinstall after bun update (2 of 2 runs here).
install: consolidate the open package manager pull requests #44968 made bun install --force, bun add <the same spec> and the isolated linker take new bytes too. install: fix regressions and older bugs found after the package manager consolidation #45307 made hasInstallScript a key of bun.lock. No tag contains either commit.
npm 11.16.0 does what this PR does: npm update after the tarball changed moves integrity, writes hasInstallScript: true and runs the new hook.

What changes, by row. These commands read a tarball again: bun update, bun update <name>, bun install --force, bun add <the same spec>.

file: and URL tarball, bun.lockb: the row records the scripts of what was read. The run and the installs that follow run them. bun pm untrusted lists them.
file: and URL tarball, both lockfiles: hasInstallScript is that of what was read, set or cleared. An install reports a blocked script of the new build.
The same bytes: a row that has the integrity and lacks hasInstallScript gets it. That is a bun.lock from before the key. A row of bun.lockb that an older version moved gets the scripts of its bytes.
git and GitHub: bun update adds hasInstallScript: true to a row that lacks it. The flag is never cleared and the scripts stay. A git row is read from its checkout in the cache. node_modules links to those files on Linux and Windows, so a lifecycle script can have edited them.

Designs weighed.

A scripts store with no condition, which is what a fresh row gets. It saves one read of the installed package.json per moved row of bun.lock. It makes bun update run the hook of the tarball where the installed package.json has the patched one: 16 of 16 tarball cells, and 4 of 4 bun update cells of a git or GitHub row with nothing new upstream. main, 1.4.3-canary.1 and bun install run the patched hook there.
One replace_package helper for this code and the folder resolver. It stores the same row and needs a list of what to keep (resolution, the id, the integrity), so a list stays.
Clearing the scripts of the row, so that the readers read the installed package.json. It rewrites rows of bun.lockb and costs every later install a read.

Not in this PR.

The linkers. A plain bun install of a row that has no integrity records the integrity there and nothing else. The next command that reads the tarball again repairs the row.
A GitHub row whose source sends another package.json under the same archive root (<owner>-<repo>-<7 hex>). On main the archive that was read is discarded when the cache has the folder. install: name a github: cache entry after its commit and the sha512 that bun.lock pins #45671 owns that row.
Which package.json decides the hooks of a patched package. A row that records scripts (a fresh install, every row of bun.lockb) runs the hooks of the tarball. A row of bun.lock runs the patched ones. The readers decide that (Scripts::get_list, get_installed_package_scripts_count). This PR changes 0 of those cells.
install: record the version of tarball and git packages in the lockfile, and audit them #46143 adds two columns at the same lines of this code, and a key to the info object of bun.lock. The PR that merges second has to update the exact objects that these tests expect.

Measurements. Debug builds and release builds of main f2de590 and of this PR. strace, perf, valgrind and bloaty are not installed here, so gdb stands in.

Writes of an existing row in append_extracted_package (gdb breakpoints), main -> PR. A registry install (95 packages, warm cache): 0 -> 0. A no-op install: 0 -> 0. A plain install with an unchanged tarball: 0 -> 0. bun update with an unchanged tarball: 0 -> 1. bun update after the tarball changed: 1 -> 2.
Code: append_extracted_package 2175 -> 2810 bytes (+635) and 438 -> 566 instructions (+128) (llvm-nm -S, llvm-objdump). Most of it is a second inlined write of the row. size text 107,401,036 -> 107,401,804, data and bss equal.
Opens of node_modules/pkg/package.json in a run that reads the tarball again (gdb catch syscall openat, hoisted and isolated). Trusted: bun.lock 2 -> 2, bun.lockb 1 -> 1. Not trusted, and the row gains the flag: 0 -> 1, the read that counts the blocked script.
Lockfile bytes. bun update with an unchanged tarball: 0 differing bytes in bun.lock, 0 in bun.lockb. bun update with a git dependency and nothing new upstream: 0 and 0. A bun.lock without hasInstallScript: 1 changed row. A bun.lockb migrated from package-lock.json, after a move: +0 bytes, byte-identical to main's.
bun.lock and a patch that edits scripts: 0 of 36 cells change. All run the patched hook on both builds (16 tarball cells with other bytes, 16 with the same bytes, 4 bun update cells of a git or GitHub row). Without a patch: 0 of 36.

Tests. 29 cases.

Case	Kinds	Count
`bun update` runs the scripts of the new build when it adds, changes or drops one, with bun.lock and with bun.lockb	`file:`, both linkers	12
`update`, `install --force`, `add <spec>` and `update pkg` record the flag and report the blocked script. The builds have a postinstall, a preinstall, a bare binding.gyp, or `gypfile: false`	`file:` and URL, both linkers	4
bun.lockb and a package that is not trusted: the report, `bun pm untrusted`, and `bun update --save-text-lockfile`	`file:` and URL, both linkers	4
A row of an older bun.lock and the same bytes, four commands. The row has no integrity, or the integrity and no flag	`file:`, both linkers	4
`bun update` runs the hook that a patch edited	`file:`, both linkers	2
git and GitHub: a bun.lock from before the key gets it. An edited git checkout does not clear it		3

Debug build of main: 25 fail. The 4 that pass are controls: 2 where the script changes and stays (bun.lock), 2 that hold the patched hook.
One clause removed at a time: the filled condition 2 fail, the scripts store 10 fail, the flag store 16 fail, the store for the same bytes 2 fail, the flag of a git or GitHub row 3 fail, a flag that is also cleared there 1 fails.
One code path serves file: and URL rows, so three of the cases run for file: only. The two that run for both hold the flag and the scripts of a URL row.

Other suites, debug build, 0 fail. bun-install 772, bun-install-lifecycle-scripts 428, bun-install-git-deps 201, GHSA-pfwx-36v6-832x 10, bun-update 175, bun-lockb 67, bun-patch 218, bun-install-patch 53, bun-pm 154, bun-lock 317, bun-update-transitive 326, bun-update-lockfile-sync 172, hoist 175, isolated-install 283, bun-add 219. Nothing ran here on Windows or macOS.

robobun · 2026-10-20T20:06:26Z

^{Updated 6:12 AM PT - Oct 22nd, 2026}

❌ @robobun, your commit 12b6213 has 1 failures in Build #130745 (All Failures):

test/js/bun/module-graph/module-graph-isolation.test.ts - code 1: } on 🍎 any x64

🧪 To try this PR locally:

bunx bun-pr 46238

That installs a local version of the PR into your bun-46238 executable, so you can run:

bun-46238 --bun

coderabbitai · 2026-10-20T20:10:28Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: f39773df-0286-4a18-9de5-944401407d83

📥 Commits

Reviewing files that changed from the base of the PR and between ea6a2c7 and e6b2388.

📒 Files selected for processing: 2

src/install/PackageManager/processDependencyList.rs
test/cli/install/bun-install-tarball-integrity.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Walkthrough

Reused packages now refresh lifecycle-script metadata under the conditions described in the package manager. New tests cover tarball, Git, and GitHub dependency updates, including metadata preservation and script execution.

Changes

Install lifecycle-script metadata

Layer / File(s)	Summary
Refresh lifecycle-script metadata `src/install/PackageManager/processDependencyList.rs`, `test/cli/install/bun-install-tarball-integrity.test.ts`	Reused-package handling updates stored scripts and install-script flags. Tests cover tarball script changes, trust and install flows, lockfile variants, and Git and GitHub dependency metadata.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to e6b23

Moved file: and URL tarball dependencies now take their lifecycle-script metadata from the newly downloaded build. Git and GitHub rows now gain a missing install-script flag. The change is narrowly scoped and has extensive tests across lockfile formats and install commands, and no concrete defect remains. It looks ready to merge after normal CI.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly describes the main change: reread tarball rows now receive lifecycle scripts from the newly read build. The wording is slightly awkward but remains specific and relevant.
Description check		The description explains the problem, fix, scope, design decisions, downsides, and verification results. It does not use the exact template headings, but it provides the required change summary and ve…

Full details: Description check

Explanation

The description explains the problem, fix, scope, design decisions, downsides, and verification results. It does not use the exact template headings, but it provides the required change summary and verification details in greater depth.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

robobun · 2026-10-20T20:14:23Z

Status

How it was reproduced: a debug build of main (f2de590) runs the tests of this PR with bun bd test test/cli/install/bun-install-tarball-integrity.test.ts. 25 of the 29 new cases fail.

With bun.lockb, a trusted file: tarball whose new build changes its preinstall runs the old script after bun update.
With bun.lock, the row of the package stays {} after the new build adds a postinstall, and the install prints no Blocked 1 postinstall.

With this PR the file passes: 194 pass, 0 fail.

PR: #46238

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

🟣 src/install/PackageManager/processDependencyList.rs — pre-existing: a tarball row that already carries an integrity but no hasInstallScript never gains it, so its untrusted postinstall is never reported as blocked. On bun update, install --force or add the tarball is re-read, but with unchanged bytes processDependencyList.rs:183 returns before the new store at :201-210; a git row (:207) and a row with no integrity do gain the flag. Fix: store has_install_script (and scripts when scripts.filled) for every re-read tarball row, equal integrity included, or set it where the linkers first record the integrity (PackageInstaller.rs:1574, Installer.rs:293). Such rows come from a plain bun install on an older bun.lock and from a yarn.lock migration (yarn.rs:786). [also at: src/install/PackageManager/processDependencyList.rs:184 - pre-existing: a tarball row whose recorded integrity already equals the file keeps a stale hasInstallScript and stale bun.lockb scripts, and no command can repair it.; src/install/PackageManager/processDependencyList.rs:206 - pre-existing: a bun.lockb tarball row that an earlier bun already moved keeps its stale scripts and hasInstallScript forever, so a trusted package keeps running the old build's postinstall on every install.]

Why this was flagged

A bun.lock written before the hasInstallScript key has a file: or URL tarball dependency with a postinstall that is not in trustedDependencies. The user upgrades bun and runs plain bun install: the row is not repinned (PackageManagerDirectories.rs:928-938), the linker records only the integrity (PackageInstaller.rs:1574-1582, isolated Installer.rs:293) and saves the lockfile. Later bun update re-reads the tarball, the extracted sha512 equals the recorded one, moved is false at processDependencyList.rs:178 and :183 returns existing before the new lines :201-210 run, so hasInstallScript stays unset for good. PackageInstaller.rs:2486 and isolated_install/Installer.rs:1325 gate the blocked-script count on that flag, so the install never prints the Blocked 1 postinstall. Run \bun pm untrusted` for details.line of install_with_manager.rs:1502. The base branch behaves the same; the PR fixes only rows whose integrity differs, and git rows on the same lockfile do get the flag from :207-209 on the samebun update`.

Verification: pre-existing. Triggering condition: a bun.lock whose tarball row has an integrity but no hasInstallScript key, followed by bun update with unchanged tarball bytes. In processDependencyList.rs if !(from_lockfile && moved) { return existing; } (:182-184) runs before the new store at :206, so the flag is never set; PackageInstaller.rs:2486 gates the blocked-script count on it.

claude

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (1):

Unresolved: 1 minor or pre-existing.

… of what was read `bun update`, `bun install --force` and `bun add <the same spec>` read a `file:` or URL tarball again. When it held other bytes, its row of the lockfile took the dependencies, the bin and the integrity of the package.json that was read, and kept the lifecycle scripts and `hasInstallScript` that it had. - bun.lockb records the scripts, so a trusted package ran the scripts of the old build in the new one, and `bun pm untrusted` listed them. - bun.lock kept the old `hasInstallScript`, so the install did not report a blocked postinstall that the new build added. The row now takes `hasInstallScript`, and the scripts when it records scripts. A row of bun.lock records none: the installed package.json, which a patch can edit, still decides what runs. The row takes both when the bytes are the same too. That repairs a row that an older version wrote. `bun update` also reads a git or GitHub dependency again at the same commit. Such a row only gets a `hasInstallScript` that it lacks, which is the case for a bun.lock from before the key was written.

claude

I reviewed the latest push and found no bugs; because it decides which lifecycle scripts a re-read tarball row records and whether an untrusted hook is reported as blocked, a human look is still worthwhile — especially since the title and description still describe a store limited to a moved row, while the code now stores scripts/hasInstallScript for every lockfile tarball row that is read again (same integrity included), which the "Downsides" paragraph lists as not done.

What was reviewed:

append_extracted_package in src/install/PackageManager/processDependencyList.rs: the new store runs only when from_lockfile, only for tarball rows, and only via repins_tarball (update / --force / add), which requires save_lockfile(), so --frozen-lockfile and plain bun install never reach it; --frozen-lockfile cannot rewrite the row.
existing.scripts is replaced only when filled (bun.lockb rows); bun.lock rows keep deferring to the installed, patchable package.json, and package.scripts.filled is always true after parse so the row stays filled.
has_install_script on the re-read package is computed by Package::parse for non-root resolvers (scripts or binding.gyp with gypfile), so the copied flag matches what a fresh install writes.
Test helpers: hostedGitRepository/githubTarball exist in harness with the sha/env shape the tests use; the git helper now drains stdout/stderr/exited concurrently and the edited-checkout case asserts the checkout is read before asserting the flag is kept.

Extended reasoning...

The change is a 17-line edit to append_extracted_package in the package manager plus about 280 lines of tests in the existing tarball-integrity file. It touches the lifecycle-script trust surface: which scripts a bun.lockb row records for a trusted package and whether bun.lock's hasInstallScript gates the "Blocked N postinstall" report, so a wrong value either runs a stale hook or silently hides a new one. The code is small and the guards trace cleanly, but this push changed the semantics from "moved rows only" to "every re-read lockfile tarball row" while the PR title and description still state the narrower behavior, and the author is a bot, so a maintainer should confirm the widened store is the intended design. The prior nits on pipe draining and the edited-checkout precondition are addressed in the code; the optional runtime nit on the debug/ASAN lane is partly mitigated by the new fileOnly skips but not otherwise answered.

Still open from earlier reviews (1):

Unresolved: 1 minor or pre-existing.

robobun · 2026-10-22T06:23:07Z

On the pre-existing finding of the review (a tarball row that has the integrity and no hasInstallScript never gains it): 12b6213 handles it for every command that reads the tarball again.

append_extracted_package now stores hasInstallScript, and the scripts when the row records them, also when the integrity is equal.
bun update, bun update <name>, bun install --force and bun add <the same spec> repair such a row. They also repair a row of bun.lockb that an older version moved.
The case "each command records the lifecycle script of a row that has the integrity and no hasInstallScript" holds it. It fails on main and without the new store.

Not changed: a plain bun install reads no tarball again. The linkers that give a row its first integrity (PackageInstaller.rs:1574, isolated_install/Installer.rs:293) still record only the integrity. That is under Downsides in the description.

The three inline nits are addressed in the same commit: the helper reads stdout, the file spawns fewer processes, and the edited-checkout case first shows that bun update reads the edited package.json.

The title and the description of the PR now describe this wider store.

github-actions Bot added the claude label Oct 20, 2026

robobun mentioned this pull request Oct 20, 2026

install: name a github: cache entry after its commit and the sha512 that bun.lock pins #45671

Draft

claude Bot reviewed Oct 20, 2026

View reviewed changes

Comment thread test/cli/install/bun-install-tarball-integrity.test.ts Resolved

Comment thread test/cli/install/bun-install-tarball-integrity.test.ts Resolved

Comment thread test/cli/install/bun-install-tarball-integrity.test.ts Resolved

robobun force-pushed the robobun/a16f05c7/moved-row-lifecycle-scripts branch from e6b2388 to 4b16aa0 Compare October 22, 2026 05:42

github-actions Bot reviewed Oct 22, 2026

View reviewed changes

Comment thread src/install/PackageManager/processDependencyList.rs Resolved

claude Bot reviewed Oct 22, 2026

View reviewed changes

robobun force-pushed the robobun/a16f05c7/moved-row-lifecycle-scripts branch from 4b16aa0 to 12b6213 Compare October 22, 2026 06:08

robobun changed the title ~~install: give a moved tarball row the lifecycle scripts of its new build~~ install: give a tarball row that is read again the lifecycle scripts of its build Oct 22, 2026

claude Bot reviewed Oct 22, 2026

View reviewed changes

robobun mentioned this pull request Oct 24, 2026

install: read the package.json of migrated git and tarball rows from the cache #46625

Open

robobun mentioned this pull request Nov 1, 2026

install: read os, cpu and libc of folder, tarball and git packages #47660

Open

Navigation Menu

Repository navigation

install: give a tarball row that is read again the lifecycle scripts of its build - #46238

install: give a tarball row that is read again the lifecycle scripts of its build#46238
robobun wants to merge 1 commit into
mainfrom
robobun/a16f05c7/moved-row-lifecycle-scripts

robobun commented Oct 20, 2026 •

edited

Loading

Uh oh!

robobun commented Oct 20, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 20, 2026 •

edited

Loading

Uh oh!

robobun commented Oct 20, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Oct 22, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robobun commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Background

Downsides

Uh oh!

robobun commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

robobun commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

robobun commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Oct 20, 2026 •

edited

Loading

robobun commented Oct 20, 2026 •

edited

Loading

coderabbitai Bot commented Oct 20, 2026 •

edited

Loading

robobun commented Oct 20, 2026 •

edited

Loading

robobun commented Oct 22, 2026 •

edited

Loading